fix(backend): GitHub API レート制限の 403 をリトライせず握り込む不具合を修正(#485)#487
Conversation
fetch_repo_tree が rate-limit の 403 を partial 走査として黙って握り込み、 リトライされずスキル証跡が欠けていた問題を修正。fetch_repos_raw の判別ロジックを 共通ヘルパ(_raise_if_rate_limited)に抽出し、fetch_repo_tree / fetch_languages / fetch_repo_file の GitHub API 呼び出し全経路で RetryableError に統一した。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors GitHub API client rate-limit handling by introducing a shared ChangesRate-limit retry handling in GitHub API client
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant fetch_repo_tree
participant _raise_if_rate_limited
participant GitHubAPI
Caller->>fetch_repo_tree: request repo tree
fetch_repo_tree->>GitHubAPI: GET /repos/{owner}/{repo}/git/trees/{branch}
GitHubAPI-->>fetch_repo_tree: response (status, headers)
fetch_repo_tree->>_raise_if_rate_limited: check response
alt rate limited (403 remaining=0 or 429)
_raise_if_rate_limited-->>Caller: raise RetryableError(retry_after)
else genuine 403 or non-200
_raise_if_rate_limited-->>fetch_repo_tree: no error
fetch_repo_tree-->>Caller: return ([], True) with warning
else success
fetch_repo_tree-->>Caller: return parsed tree
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/tests/test_github_api_client.py (1)
91-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTests look solid for
fetch_repo_treeandfetch_languages.One minor gap:
_raise_if_rate_limitedwas also wired intofetch_repo_file(line 263), but no regression test covers that path. Consider adding a test mirroring thefetch_repo_treepattern to verify rate-limited 403/429 raisesRetryableErrorand genuine 403 returnsNone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_github_api_client.py` around lines 91 - 130, Add regression coverage for the `fetch_repo_file` path, since it also calls `_raise_if_rate_limited` but is not exercised here. Mirror the existing `fetch_repo_tree`/`fetch_languages` style in `test_github_api_client.py` by adding tests that confirm `fetch_repo_file` raises `RetryableError` for rate-limited 403/429 responses and still returns `None` for a genuine 403 with remaining quota. Use the existing helpers like `_client_with_status` and `_run` to keep the new tests consistent with the current suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/tests/test_github_api_client.py`:
- Around line 91-130: Add regression coverage for the `fetch_repo_file` path,
since it also calls `_raise_if_rate_limited` but is not exercised here. Mirror
the existing `fetch_repo_tree`/`fetch_languages` style in
`test_github_api_client.py` by adding tests that confirm `fetch_repo_file`
raises `RetryableError` for rate-limited 403/429 responses and still returns
`None` for a genuine 403 with remaining quota. Use the existing helpers like
`_client_with_status` and `_run` to keep the new tests consistent with the
current suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dd0cc1dd-8f83-4dff-971c-c19593ff6430
📒 Files selected for processing (2)
backend/app/services/intelligence/github/api_client.pybackend/tests/test_github_api_client.py
fetch_repo_file も _raise_if_rate_limited を呼ぶが未テストだったため、 fetch_repo_tree / fetch_languages と同じスタイルで - レート制限 403 → RetryableError - 429 → RetryableError - 権限 403(残量あり)→ None(当該 manifest スキップ) の 3 ケースを追加。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
fetch_repo_treeが GitHub API のレート制限(403 +X-RateLimit-Remaining:0/ 429)を「1 リポの部分走査(partial)」として黙って握り込み、リトライされずスキル証跡が静かに欠けていた不具合を修正しました(#485。#478 の実データ計測中に発見した既知事項の follow-up)。fetch_repos_rawに既にあったレート制限判別ロジックを共通ヘルパ_raise_if_rate_limitedに抽出fetch_repo_tree/fetch_languages/fetch_repo_file(同一ホットパスの兄弟 fetch)の全経路でレート制限をRetryableError(retry_after付き)として統一し、連携タスク全体をリトライさせるTest Plan
make cipass(lint / typecheck / test / build)fetch_repos_rawの既存リトライ挙動(test_retry_flow.py等 88 件)に回帰なしNotes
fetch_repo_treeに加え、同じ握り込みバグを持つfetch_languages/fetch_repo_fileも同一 PR で修正(tree だけ直すと mid-repo でのレート制限時に同じ問題が残るため)Closes #485
🤖 Generated with Claude Code
Summary by CodeRabbit